Skip to content

Conversation

@haampie
Copy link
Contributor

@haampie haampie commented Sep 30, 2025

Closes #13916

Alternative to #13918

  • Remove support for ?highlight= query param as a (manual) fallback for search highlighting.

While at it:

  • Simplify document.querySelectorAll("div.body")[0] to document.querySelector("div.body")
  • Simplify document.querySelector("body") with document.body
  • Replace setTimeout with requestAnimationFrame

Purpose

This PR removes the ?highlight= related code, for two reasons:

  1. Sphinx does not use ?highlight= links internally.
  2. Browser native support for Text Fragments #:~:text=... is a decent replacement for ?highlight=... for adding highlighting manually.
  3. The current logic runs window.history.replaceState unconditionally. This has an unpleasant side effect: it removes #:~:text=... fragments from the URL, making it hard to purposefully share URLs that include highlighted text.

References

* Do not use `?highlight=` query param as a fallback for search
  highlighting. All major browsers support `#:~:text=` natively now, and
  Sphinx does not link from search results with `?highlight=` URLs, it
  only uses `localStorage`.
* Simplify `document.querySelectorAll("div.body")[0]` to
  `document.querySelector("div.body")`
* Simplify `document.querySelector("body")` with `document.body`
* Replace `setTimeout` with `requestAnimationFrame`
@haampie haampie changed the title sphinx_highlight.js: remove highlight query param handling sphinx_highlight.js: remove ?highlight= query param handling Sep 30, 2025
haampie added a commit to spack/spack that referenced this pull request Oct 7, 2025
Use Sphinx 8.2.3 with this patch:
sphinx-doc/sphinx#13921, so that it's easier to
link particular paragraphs in the text using Text Fragments.
haampie added a commit to spack/spack that referenced this pull request Oct 7, 2025
Use Sphinx 8.2.3 with this patch:
sphinx-doc/sphinx#13921, so that it's easier to
link particular paragraphs in the text using Text Fragments.

Signed-off-by: Harmen Stoppels <[email protected]>
@mgeier
Copy link
Contributor

mgeier commented Nov 8, 2025

  • Sphinx does not use ?highlight= links internally.

It depends on what you mean by "use". It supports them in so far as if you append ?highlight=..., the given word will be highlighted.

It doesn't "use" it in so far as it doesn't show URLs with ?highlight=... when you click on a search result.

Until a few years ago, it was added to the URL, and I made a PR for it to be removed when clicking "Hide Search Matches": #9551.

Some people didn't like that ?highlight=... was added at all (#10833) and this behavior was removed in #10854.

I still think that this was a mistake and that removing ?highlight=... when clicking "Hide Search Matches" is the better solution. IMHO the real problem is that many themes show "Hide Search Matches" not prominent enough (or not at all).

  • Browser native support for Text Fragments #:~:text=... is a decent replacement for ?highlight=... for adding highlighting manually.

I don't think it is a decent replacement, since it only highlights the first occurrence, while ?highlight=... highlights all occurrences, which is what I would expect.

@haampie
Copy link
Contributor Author

haampie commented Nov 8, 2025

I don't think it is a decent replacement, since it only highlights the first occurrence, while ?highlight=... highlights all occurrences, which is what I would expect.

So, ?highlight=... is clearly only useful for sharing links, right? When not sharing, you'd do Ctrl+F instead of modifying a URL and reloading the page.

If it's for sharing links, then I would argue #:~:text=... is superior because it scrolls the fragment into view, making it immediately clear what to read from the page. If you open a link with ?highlight= and the keyword is in multiple places, it's unclear which part is relevant, and it requires the user to scroll to the first match (they may use Ctrl+F again?)

That to me suggests there's not a good reason to keep the feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sphinx_highlight.js removes text fragment from URL

2 participants